chore(go): add plain text migration examples #1966
Conversation
…ase-encryption-sdk-dynamodb into plaintextmigration
| MigrationStep1(kmsKeyID, tableName, partitionKey, sortKeys[1]) | ||
| // When: Execute Step 2 with sortReadValue=1, Then: Success (i.e. can read encrypted values) | ||
| MigrationStep2(kmsKeyID, tableName, partitionKey, sortKeys[1]) | ||
|
|
There was a problem hiding this comment.
Can we also add testcase to write with step 3 and read with step 2? also for step 1_test.go
see similar test in Python.
There was a problem hiding this comment.
Thanks. It was a very good finding.
I did it in this commit: 4eb1d8a. I also figured that a change in local to handle error was not committed. So, this Commit include that too.
| make test_go | ||
|
|
||
| - name: Test Examples | ||
| - name: Run and Test Examples |
There was a problem hiding this comment.
Why run and test both? Since it's an example, I don't think we need to assert all cases via unit tests, but if it's already in then no worries.
There was a problem hiding this comment.
For migrations, I needed to run a lot of combination of steps that made sense of run as a unit test. For all other test, just running a round trip was enough but migration is different so it was not enough
| Item: item, | ||
| } | ||
|
|
||
| _, err = ddb.PutItem(context.TODO(), &putInput) |
There was a problem hiding this comment.
Should we use a plain vanilla client to demonstrate that we actually aren't doing any encryption and can read unencrypted items using our client?
There was a problem hiding this comment.
We do this in Test.
This is one combination of the test: In step 1 test, we write with vanilla DDB client and read with intercepted DDB client. In step 0 test, we write with intercepted DDB client and read with the vanilla one.
There was a problem hiding this comment.
We have a blocker on test but this PR is good to go: https://quip-amazon.com/dVbiAID0wjHq#DLY9DAFeZPq
| Generator: &kmsKeyID, | ||
| } | ||
| kmsKeyring, err := matProv.CreateAwsKmsMrkMultiKeyring(context.Background(), keyringInput) | ||
| utils.HandleError(err) |
|
|
||
| // We return this error because we run test against the error. | ||
| // When used in production code, you can decide how you want to handle errors. | ||
| if err != nil { |
There was a problem hiding this comment.
Why not use utils here as well?
There was a problem hiding this comment.
Before writing migration example, I thought panic on error everywhere is good enough but with all the test combinations in migration test, it was not good enough. I think many in the team will still have a thought that we panic on every example. So, to make it visually distinct immediately visible in the code, I did not use utils.
| if partitionKeyValue != result.Item["partition_key"].(*types.AttributeValueMemberS).Value { | ||
| panic("Decrypted item does not match original item") | ||
| } | ||
| if encryptedAndSignedValue != result.Item["attribute1"].(*types.AttributeValueMemberS).Value { |
There was a problem hiding this comment.
What about other attributes? Maybe add comment if we don't want full assertion.
There was a problem hiding this comment.
Will this change look good to you? https://github.com/aws/aws-database-encryption-sdk-dynamodb/pull/1966/files#r2255583436
I will apply this change to other places as well.
There was a problem hiding this comment.
We decided to test all attribute.
| sortKeys := []string{"0", "1", "2", "3"} | ||
|
|
||
| // Successfully executes Step 1 | ||
| err := MigrationStep1(kmsKeyID, tableName, partitionKey, sortKeys[1]) |
There was a problem hiding this comment.
Since we aren't mocking anything here, what's the difference between this and run?
There was a problem hiding this comment.
We run this only once with the test.
|
|
||
| func AssertServiceError(err error, service string, operation string, errorMessage string) { | ||
| if err == nil { | ||
| panic("Expected error but got no error") |
There was a problem hiding this comment.
Don't we want to have this function as no-op in case of no errors? In that way, we can always call this instead of figuring out when to call this.
There was a problem hiding this comment.
Not really. Every time we call this we need to have an Error. If we don't get an error then that means something is wrong.
| HandleError(err) | ||
|
|
||
| // Create DynamoDB client | ||
| client := dynamodb.NewFromConfig(cfg) |
There was a problem hiding this comment.
Or you could just use the already created client :)
There was a problem hiding this comment.
This functions is used where we don't have access of the ddb client. So, I am re-creating it. We also re-create ddb client in every example so that examples are easier to follow.
* **java:** drop hkdf offset method ([#2011](#2011)) ([b8f29f9](b8f29f9)) * Add go to smithy dependencies ([#1986](#1986)) ([066c0ca](066c0ca)) * add managed policy to cfn template ([#2012](#2012)) ([915183d](915183d)) * bump mpl ([#1953](#1953)) ([4e295f9](4e295f9)) * bump MPL and reuse release script from MPL ([#1963](#1963)) ([eb52fca](eb52fca)) * bump mpl and smithy dafny ([#1982](#1982)) ([c71fc82](c71fc82)) * bump mpl to allow nightly build ([#1984](#1984)) ([208eb31](208eb31)) * **cfn:** add DescribeTable permission ([#1952](#1952)) ([f24a0d3](f24a0d3)) * **CI:** add slack notifications ([#1964](#1964)) ([139c856](139c856)) * **CI:** fix dafny_interop to setup_net ([#1965](#1965)) ([e7a5a02](e7a5a02)) * **CI:** remove redundant dotnet run ([#2038](#2038)) ([09c3266](09c3266)) * **dafny:** bump MPL and update mutable map ([#1974](#1974)) ([e9ea870](e9ea870)) * **dafny:** bump smithy dafny ([#1971](#1971)) ([85309a0](85309a0)) * **deps:** update aws-lc-sys requirement ([#2019](#2019)) ([c397bf2](c397bf2)) * fix non prod daily CI for Go ([#2006](#2006)) ([f8d8f08](f8d8f08)) * fix non-prod.yml ([#2007](#2007)) ([43b5163](43b5163)) * **go:** Add client supplier and basic put get example ([#1950](#1950)) ([2e36410](2e36410)) * **go:** add complex example in searchable encryptions ([#1955](#1955)) ([012ebf9](012ebf9)) * **go:** Add Go release workflow and script ([#1958](#1958)) ([2fe77ab](2fe77ab)) * **go:** add performance benchmark ([#1983](#1983)) ([928e318](928e318)) * **go:** add plain text migration examples ([#1966](#1966)) ([c8796d9](c8796d9)) * **go:** add Searchable Encryption Examples ([#1951](#1951)) ([07bdbe7](07bdbe7)) * **go:** Add various keyrings examples in Go ([#1948](#1948)) ([ffb0c38](ffb0c38)) * **go:** fix nit in go-release workflow ([#1978](#1978)) ([45f7f15](45f7f15)) * **go:** Update ci_test_go.yml ([#1970](#1970)) ([b373067](b373067)) * **go:** update go test matrix and clean up setup ([#1959](#1959)) ([7dd705b](7dd705b)) * **java:** Allow local testing ([#1947](#1947)) ([bf5a106](bf5a106)) * **java:** fix GetEncryptedDataKeyDescription java Example ([#1973](#1973)) ([ba8fcb7](ba8fcb7)) * move go out of pull/push/daily into its own ([#2005](#2005)) ([1b961b5](1b961b5)) * **net:** Add plaintext to encrypted table migration example ([#1976](#1976)) ([814acbf](814acbf)) * remove test against latest mpl version ([#2008](#2008)) ([728158e](728158e)) * **rust:** add plaintext to encrypted table migration examples ([#1977](#1977)) ([5286619](5286619)) * **rust:** add simple performance benchmarks ([#2027](#2027)) ([a5bdeb7](a5bdeb7)) * **rust:** bump mpl to remove rust warnings ([#1993](#1993)) ([4d16a62](4d16a62)) * **rust:** fips feature ([#1980](#1980)) ([516fd3d](516fd3d)) * **rust:** improved blocking ([#2030](#2030)) ([162c86c](162c86c)) * **rust:** prepare release of 1.2.0 ([#1981](#1981)) ([5a2569a](5a2569a)) * **rust:** release 1.2.1 ([#2004](#2004)) ([47f6cd9](47f6cd9)) * **rust:** release 1.2.2 ([#2035](#2035)) ([cacda77](cacda77)) * **rust:** use Dafny 4.10.0 for Rust codegen ([#1995](#1995)) ([95168a2](95168a2)) * **TestVectors:** add config tests for new attributes ([#1967](#1967)) ([4017a97](4017a97)) * update check-files ([#2041](#2041)) ([0bafe07](0bafe07)) * update mpl ([#1991](#1991)) ([fed53bf](fed53bf)) * use macos-14 for CI ([#1998](#1998)) ([e0d2b77](e0d2b77))
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.